Skip to content

[Doctrine] SearchFilter - Use abitrary index instead of value#5079

Merged
soyuka merged 1 commit into
api-platform:3.0from
Korbeil:fix/parameter-not-set
Oct 24, 2022
Merged

[Doctrine] SearchFilter - Use abitrary index instead of value#5079
soyuka merged 1 commit into
api-platform:3.0from
Korbeil:fix/parameter-not-set

Conversation

@Korbeil

@Korbeil Korbeil commented Oct 21, 2022

Copy link
Copy Markdown
Contributor
Q A
Branch? 3.0
Tickets N/A
License MIT
Doc PR N/A

When using a search filter with multiple values that have the same lowercase identity, for example:

array:2 [▼
  0 => "rayon bleu"
  1 => "Rayon Bleu"
]

SearchFilter will lowercase the value and use it as index key, which makes it send only one parameter to Doctrine when it needs two of them.

I choosed to leave the arbitrary array index so we don't have this kind of issues anymore.

@Korbeil
Korbeil force-pushed the fix/parameter-not-set branch 3 times, most recently from 0c4a2ed to 729aca1 Compare October 21, 2022 10:04
@alanpoulain

Copy link
Copy Markdown
Member

Could you please add unit tests?

@Korbeil
Korbeil force-pushed the fix/parameter-not-set branch 2 times, most recently from fa45aac to 9ca1f93 Compare October 21, 2022 12:40
@Korbeil

Korbeil commented Oct 21, 2022

Copy link
Copy Markdown
Contributor Author

Could you please add unit tests?

Is that enough ? Not sure what I could add more 🤔

@alanpoulain

Copy link
Copy Markdown
Member

Is that enough ? Not sure what I could add more thinking

Yes, that's enough 🙂 You need to change the SearchFilterTest for MongoDB too (sorry for that, please tell me if you want me to do it).

@bastnic

bastnic commented Oct 21, 2022

Copy link
Copy Markdown
Contributor

nice catch

@soyuka

soyuka commented Oct 21, 2022

Copy link
Copy Markdown
Member

how was this in 2.6 ? I think that we could fix this on 3.0 instead, 2.7 should have only bc-layer related fixes. Unless this was wrongly introduced in 2.7 we may fix it here.

@Korbeil
Korbeil force-pushed the fix/parameter-not-set branch from 9ca1f93 to 3b89b7e Compare October 21, 2022 13:43
@Korbeil

Korbeil commented Oct 21, 2022

Copy link
Copy Markdown
Contributor Author

how was this in 2.6 ? I think that we could fix this on 3.0 instead, 2.7 should have only bc-layer related fixes. Unless this was wrongly introduced in 2.7 we may fix it here.

I do think this was here since a long time, I'll change the branch to 3.0 then.

@Korbeil
Korbeil force-pushed the fix/parameter-not-set branch from 3b89b7e to b4a9ba7 Compare October 21, 2022 14:13
@Korbeil
Korbeil changed the base branch from 2.7 to 3.0 October 21, 2022 14:13
@soyuka
soyuka merged commit 7044c5a into api-platform:3.0 Oct 24, 2022
@soyuka

soyuka commented Oct 24, 2022

Copy link
Copy Markdown
Member

Thanks @Korbeil !

@Korbeil
Korbeil deleted the fix/parameter-not-set branch October 24, 2022 09:14
soyuka added a commit that referenced this pull request Nov 4, 2022
* fix: update yaml extractor test file coding standard (#5068)

* fix(graphql): add clearer error message when TwigBundle is disabled but graphQL clients are enabled (#5064)

* fix(metadata): add class key in payload argument resolver (#5067)

* fix: add class key in payload argument resolver

* add null if everything else goes wrong

* fix: upgrade command remove ApiSubresource attribute  (#5049)

Fixes #5038

* fix(doctrine): use abitrary index instead of value (#5079)

* fix: uri template should respect rfc 6570 (#5080)

* fix: remove @internal annotation for Operations (#5089)

See #5084

* fix(metadata): define a name on a single operation (#5090)

fixes #5082

* fix(metadata): deprecate when user decorates in legacy mode (#5091)

fixes #5078

* fix(graphql): use right nested operation (#5102)

* Revert "fix(graphql): use right nested operation (#5102)" (#5111)

This reverts commit 44337dd.

* fix(graphql): always allow to query nested resources (#5112)

* fix(graphql): always allow to query nested resources

* review

Co-authored-by: Alan Poulain <contact@alanpoulain.eu>

* chore: php-cs-fixer update

* fix: only alias if exists for opcache preload

Fixes api-platform/api-platform#2284 (#5110)

Co-authored-by: Liviu Mirea <liviu.mirea@wecodepixels.com>

* chore: php-cs-fixer update (#5118)

* chore: php-cs-fixer update

* chore: php-cs-fixer update

* fix(metadata): upgrade script keep operation name (#5109)

origin: #5105

Co-authored-by: WilliamPeralta <william.peralta18@gmail.com>

* chore: v2.7.3 changelog

* chore: v3.0.3 changelog

Co-authored-by: helyakin <CourcierMarvin@gmail.com>
Co-authored-by: ArnoudThibaut <thibaut.arnoud@gmail.com>
Co-authored-by: davy-beauzil <38990335+davy-beauzil@users.noreply.github.com>
Co-authored-by: Baptiste Leduc <baptiste.leduc@gmail.com>
Co-authored-by: Xavier Laviron <norival@users.noreply.github.com>
Co-authored-by: Alan Poulain <contact@alanpoulain.eu>
Co-authored-by: Liviu Cristian Mirea-Ghiban <contact@liviucmg.com>
Co-authored-by: Liviu Mirea <liviu.mirea@wecodepixels.com>
Co-authored-by: WilliamPeralta <william.peralta18@gmail.com>
alexndlm added a commit to eonx-com/easy-monorepo that referenced this pull request Nov 7, 2022
alexndlm added a commit to eonx-com/easy-monorepo that referenced this pull request Nov 18, 2022
alexndlm added a commit to eonx-com/easy-monorepo that referenced this pull request Jan 13, 2023
natepage pushed a commit to eonx-com/easy-monorepo that referenced this pull request May 21, 2023
* -create AdvancedSearchFilter

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -rollback

* -sync with api-platform/core#5079

* -fix ecs

* -fix typo

* -refactoring

* -fix ecs

* -refactoring

* -fix phpstan

* -refactoring

* -fix ecs

* -update composer.json

* -update composer.json

* -rename Stubs to Fixtures

* -rename Stubs to Fixtures

* -rename Stubs to Fixtures

* -refactoring

* -refactoring

* -fix phpstan

* -rollback changes

* -fix deps
natepage pushed a commit to eonx-com/easy-api-platform that referenced this pull request May 21, 2023
* -create AdvancedSearchFilter

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -rollback

* -sync with api-platform/core#5079

* -fix ecs

* -fix typo

* -refactoring

* -fix ecs

* -refactoring

* -fix phpstan

* -refactoring

* -fix ecs

* -update composer.json

* -update composer.json

* -rename Stubs to Fixtures

* -rename Stubs to Fixtures

* -rename Stubs to Fixtures

* -refactoring

* -refactoring

* -fix phpstan

* -rollback changes

* -fix deps
natepage pushed a commit to eonx-com/easy-core that referenced this pull request May 21, 2023
* -create AdvancedSearchFilter

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -fix dependencies

* -rollback

* -sync with api-platform/core#5079

* -fix ecs

* -fix typo

* -refactoring

* -fix ecs

* -refactoring

* -fix phpstan

* -refactoring

* -fix ecs

* -update composer.json

* -update composer.json

* -rename Stubs to Fixtures

* -rename Stubs to Fixtures

* -rename Stubs to Fixtures

* -refactoring

* -refactoring

* -fix phpstan

* -rollback changes

* -fix deps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants